fix(gui): ignore stale provider model fetch results after provider switch by shaun0927 · Pull Request #12151 · continuedev/continue

shaun0927 · 2026-04-16T15:29:13Z

Summary

ignore late dynamic-model fetch results after the selected provider changes
keep the fix scoped to AddModelForm rather than introducing global request cancellation
add a focused regression test that reproduces the in-flight fetch + provider switch race

Why this change

PR #12046 introduced dynamic provider model fetching. That flow works, but the current callback in gui/src/forms/AddModelForm.tsx closes over selectedProvider, so a request started for provider A can still update fetchedModelsList after the user has switched to provider B.

That leaves the Add Model UI in an inconsistent state where stale models from the previous provider appear under the newly selected provider.

This PR fixes that by tracking the currently selected provider at resolution time and ignoring outdated async results.

Test plan

cd gui && npm test -- --run src/forms/AddModelForm.dynamicFetchRace.test.tsx
Reproduced the stale-result leak locally before the fix using the focused regression test
Verified the same test passes after the fix
Full GUI test suite
Manual validation in VS Code / JetBrains

Dynamic provider-model fetching allows users to switch providers while a request is still in flight. The Add Model form captured the selected provider in the callback closure, so a late response from the previous provider could still replace `fetchedModelsList` after the UI had moved on. Track the latest provider in a ref and ignore outdated async results. Add a focused regression test that reproduces the race by resolving an OpenAI fetch after switching the UI to Anthropic. Constraint: Keep the fix limited to the validated GUI race and regression coverage Rejected: Cancel in-flight requests globally | more invasive than needed for the current flow Confidence: high Scope-risk: narrow Reversibility: clean Directive: If the UI later supports multiple concurrent fetches per provider, switch from provider tracking to request-token invalidation Tested: cd gui && npm test -- --run src/forms/AddModelForm.dynamicFetchRace.test.tsx Not-tested: Full gui test suite; lint; typecheck; manual VS Code / JetBrains verification

github-actions · 2026-04-16T15:29:25Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

cubic-dev-ai

No issues found across 2 files

chatgpt-codex-connector · 2026-04-16T15:38:59Z

💡 Codex Review

continue/core/llm/fetchModels.ts

Lines 227 to 231 in 08e0a7b

    
           const llm = llmFromProviderAndOptions(provider, { 
        
             apiKey, 
        
             apiBase: apiBase || defaultApiBase, 
        
             model: "", 
        
           });

Pass provider-specific fields when building model fetch client

fetchProviderModelsViaListModels creates the provider client with only apiKey, apiBase, and model: "", so provider-specific required options are never forwarded. This breaks dynamic model fetching for providers like Azure that need additional fields (for example deployment/apiVersion/apiType): Azure falls back to an empty deployment and builds a /openai/deployments//models endpoint, so the fetch path fails even when the form is filled out correctly.

continue/gui/src/forms/AddModelForm.tsx

Lines 58 to 60 in 08e0a7b

    
           useEffect(() => { 
        
             selectedProviderRef.current = selectedProvider.provider; 
        
           }, [selectedProvider]);

Update provider race guard synchronously on provider change

The stale-response guard relies on selectedProviderRef, but this ref is updated in a passive useEffect. That leaves a timing window right after switching providers where an older in-flight request can resolve before the effect runs, still see the previous provider in the ref, and incorrectly write stale models into state. This means the provider-switch race can still reproduce under fast interactions.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

shaun0927 · 2026-04-16T15:52:12Z

I have read the CLA Document and I hereby sign the CLA

shaun0927 · 2026-04-16T15:53:30Z

recheck

The first fix guarded late async results, but the provider ref still moved in a passive effect. Under a fast provider switch, an older in-flight request could resolve before that effect ran and still see the stale provider value. Update the ref synchronously at provider-selection time and format the regression test so Prettier passes in CI. Constraint: Keep the follow-up limited to the bot-confirmed race window and formatting failure Rejected: Expand this PR to address provider-specific listModels config gaps | unrelated to the validated GUI race and would broaden scope Confidence: high Scope-risk: narrow Reversibility: clean Directive: If provider selection can change from additional code paths later, centralize provider-ref synchronization instead of relying on one callback Tested: cd gui && npm test -- --run src/forms/AddModelForm.dynamicFetchRace.test.tsx Tested: ./node_modules/.bin/prettier --check gui/src/forms/AddModelForm.tsx gui/src/forms/AddModelForm.dynamicFetchRace.test.tsx --ignore-path .gitignore --ignore-path .prettierignore Not-tested: Full GUI test suite; upstream CI rerun after push; JetBrains/e2e workflows

shaun0927 · 2026-04-16T16:02:42Z

Addressed the provider-switch timing window in fbd092c by updating the provider ref synchronously when the selection changes, and formatted the regression test so the Prettier check should pass on the rerun.

I did not fold the fetchProviderModelsViaListModels feedback into this PR because it affects a broader core model-fetch path and is not required to fix the validated Add Model race tracked in #12150. I want to keep this PR scoped to the user-visible stale-result leak and its regression coverage.

chatgpt-codex-connector · 2026-04-16T16:11:53Z

💡 Codex Review

continue/gui/src/forms/AddModelForm.tsx

Lines 132 to 134 in fbd092c

    
           if (!selectedProvider.tags?.includes(ModelProviderTags.RequiresApiKey)) { 
        
             formMethods.setValue("apiKey", ""); 
        
           }

Clear API key when switching keyed providers

This effect only clears apiKey when the newly selected provider does not require a key, but handleFetchModels later sends the current apiKey to whatever provider is selected. If a user switches from one keyed provider to another (for example OpenAI to Anthropic) and clicks fetch before replacing the field, the previous provider's secret is transmitted to the wrong API. This is a cross-provider credential leak path introduced by dynamic model fetching; reset or scope API keys per provider on every provider switch.

continue/core/llm/fetchModels.ts

Line 233 in fbd092c

return modelIds.map((id) => ({ name: id }));

Filter fetched model IDs to chat-capable entries

This returns every ID from llm.listModels() as a selectable chat model without any modality/capability filtering. For OpenAI-compatible providers, /models commonly includes non-chat IDs (embeddings, audio, moderation, etc.), so users can add an invalid chat model and later hit runtime failures during completion. Filter these results to chat-capable models (or provider-specific allowlists) before exposing them in Add Chat Model.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…del fetches Codex surfaced a real follow-up risk in the Add Model flow: after switching between two providers that both require API keys, the form could retain the previous provider's secret long enough for a fetch click to send it to the new provider. This clears the shared API key field on provider changes and adds a focused regression test that proves a keyed-to-keyed switch leaves no stale key behind for the next fetch. Constraint: Keep the follow-up limited to the credible cross-provider credential leak in the existing GUI flow Rejected: Expand this PR to broader listModels capability filtering or provider-specific fetch config forwarding | those are separate fetch-path concerns and would widen the validated race-fix scope Confidence: high Scope-risk: narrow Reversibility: clean Directive: If the form later stores provider-specific secrets simultaneously, replace this blanket reset with per-provider key scoping rather than reusing one shared apiKey field Tested: cd gui && npm test -- --run src/forms/AddModelForm.dynamicFetchRace.test.tsx Tested: ./node_modules/.bin/prettier --check gui/src/forms/AddModelForm.tsx gui/src/forms/AddModelForm.dynamicFetchRace.test.tsx --ignore-path .gitignore --ignore-path .prettierignore Not-tested: Full GUI/e2e test matrix; provider-specific dynamic fetches against live APIs

shaun0927 · 2026-04-17T04:31:24Z

Addressed the remaining in-scope Codex P1 in 1078373: switching between two providers that both require API keys now clears the shared apiKey field before another fetch can run, and the regression test now covers that keyed-to-keyed switch.

I still left the broader fetchProviderModelsViaListModels / chat-capability filtering feedback out of this PR on purpose, since those touch wider model-fetch semantics beyond the validated stale-result race and follow-up credential-leak fix tracked here.

cubic-dev-ai

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="gui/src/forms/AddModelForm.tsx">

<violation number="1" location="gui/src/forms/AddModelForm.tsx:132">
P2: Clearing the API key on every provider change discards valid credentials when switching between API-key providers.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}

chatgpt-codex-connector · 2026-04-17T04:40:53Z

💡 Codex Review

continue/gui/src/forms/AddModelForm.tsx

Lines 58 to 60 in 1078373

    
           useEffect(() => { 
        
             setFetchedModelsList([]); 
        
           }, [selectedProvider]);

Clear fetch lock when provider selection changes

When a model fetch is started for provider A and the user switches to provider B before it resolves, this effect only clears fetchedModelsList and leaves isFetchingModels true. Because the fetch button is gated by disabled={isFetchingModels}, provider B cannot fetch models until provider A's request completes (and if that request hangs, B stays blocked indefinitely), even though stale results are ignored via selectedProviderRef.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

cubic identified a real follow-up usability bug in the Add Model race fix: switching providers cleared stale results but left the previous fetch lock active, so the newly selected provider could stay blocked until the old request finished. If that earlier request hung, the new provider could be stuck indefinitely even though stale results were already ignored. This invalidates in-flight fetch generations on provider change, clears the fetch lock for the new provider, and makes `finally` unlock only the currently active fetch. The regression test now proves a pending OpenAI request no longer blocks an immediate Anthropic fetch after switching. Constraint: Keep the follow-up limited to the provider-switch fetch lock identified by cubic Rejected: Leave `isFetchingModels` tied to the old request | new provider remains blocked until stale fetch resolves Rejected: Broaden this PR to unrelated model-list filtering/fetchModels core concerns | widens scope beyond the validated GUI race path Confidence: high Scope-risk: narrow Reversibility: clean Directive: Any future async fetch guard here should invalidate both stale results and stale loading state together Tested: cd gui && npm test -- --run src/forms/AddModelForm.dynamicFetchRace.test.tsx Tested: cd gui && npx prettier --check src/forms/AddModelForm.tsx src/forms/AddModelForm.dynamicFetchRace.test.tsx --ignore-path ../.gitignore --ignore-path ../.prettierignore Not-tested: Full GUI test suite; full PR CI rerun; manual VS Code / JetBrains validation

shaun0927 · 2026-04-17T04:53:55Z

Follow-up pushed in 9deaa89 for the latest review finding.

cubic found that provider switches were still inheriting the previous fetch lock even after stale results were ignored
provider changes now invalidate stale fetch generations and release the lock for the newly selected provider immediately
added regression coverage for the pending-fetch + provider-switch + immediate-refetch path

Validation:

cd gui && npm test -- --run src/forms/AddModelForm.dynamicFetchRace.test.tsx
cd gui && npx prettier --check src/forms/AddModelForm.tsx src/forms/AddModelForm.dynamicFetchRace.test.tsx --ignore-path ../.gitignore --ignore-path ../.prettierignore

@cubic-dev-ai please re-run review on the latest head.

cubic-dev-ai · 2026-04-17T04:54:06Z

Follow-up pushed in 9deaa89 for the latest review finding.

cubic found that provider switches were still inheriting the previous fetch lock even after stale results were ignored

provider changes now invalidate stale fetch generations and release the lock for the newly selected provider immediately

added regression coverage for the pending-fetch + provider-switch + immediate-refetch path
...

@shaun0927 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

1 issue found across 2 files

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="gui/src/forms/AddModelForm.tsx">

<violation number="1" location="gui/src/forms/AddModelForm.tsx:81">
P2: Stale fetch results can be applied again after switching away and back to the same provider because the response is guarded only by provider identity, not the request generation.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}

cubic-dev-ai · 2026-04-17T04:57:49Z

-      setFetchedModelsList((prev) =>
-        selectedProvider.provider === providerAtFetchTime ? models : prev,
-      );
+      if (selectedProviderRef.current === providerAtFetchTime) {


P2: Stale fetch results can be applied again after switching away and back to the same provider because the response is guarded only by provider identity, not the request generation.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At gui/src/forms/AddModelForm.tsx, line 81: <comment>Stale fetch results can be applied again after switching away and back to the same provider because the response is guarded only by provider identity, not the request generation.</comment> <file context> @@ -72,13 +78,15 @@ export function AddModelForm({ - setFetchedModelsList((prev) => - selectedProvider.provider === providerAtFetchTime ? models : prev, - ); + if (selectedProviderRef.current === providerAtFetchTime) { + setFetchedModelsList(models); + } </file context>

Suggested change

if (selectedProviderRef.current === providerAtFetchTime) {

if (

selectedProviderRef.current === providerAtFetchTime &&

fetchGenerationRef.current === fetchGeneration

) {

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9deaa89bc9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-17T05:00:39Z

+      if (selectedProviderRef.current === providerAtFetchTime) {
+        setFetchedModelsList(models);


Guard fetched model updates by request generation

This update only checks selectedProviderRef before applying results, so two concurrent fetches for the same provider can still race: if a user clicks refresh twice (or changes API key/API base and refetches), the older request may resolve last and overwrite the newer model list. That reintroduces stale models in the picker even though fetchGenerationRef is already tracked; the generation check should also gate setFetchedModelsList.

Useful? React with 👍 / 👎.

shaun0927 requested a review from a team as a code owner April 16, 2026 15:29

shaun0927 requested review from sestinj and removed request for a team April 16, 2026 15:29

github-project-automation Bot added this to Issues and PRs Apr 16, 2026

github-project-automation Bot moved this to Todo in Issues and PRs Apr 16, 2026

dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Apr 16, 2026

cubic-dev-ai Bot reviewed Apr 16, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Apr 17, 2026

View reviewed changes

Comment thread gui/src/forms/AddModelForm.tsx

cubic-dev-ai Bot reviewed Apr 17, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Apr 17, 2026

View reviewed changes

-      if (selectedProviderRef.current === providerAtFetchTime) {
+      if (
+        selectedProviderRef.current === providerAtFetchTime &&
+        fetchGenerationRef.current === fetchGeneration
+      ) {

		if (selectedProviderRef.current === providerAtFetchTime) {
		setFetchedModelsList(models);

Conversation

shaun0927 commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why this change

Test plan

Uh oh!

github-actions Bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot commented Apr 16, 2026

💡 Codex Review

Uh oh!

shaun0927 commented Apr 16, 2026

Uh oh!

shaun0927 commented Apr 16, 2026

Uh oh!

shaun0927 commented Apr 16, 2026

Uh oh!

chatgpt-codex-connector Bot commented Apr 16, 2026

💡 Codex Review

Uh oh!

shaun0927 commented Apr 17, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Apr 17, 2026

💡 Codex Review

Uh oh!

shaun0927 commented Apr 17, 2026

Uh oh!

cubic-dev-ai Bot commented Apr 17, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai Bot Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

shaun0927 commented Apr 16, 2026 •

edited

Loading

github-actions Bot commented Apr 16, 2026 •

edited

Loading

cubic-dev-ai Bot Apr 17, 2026 •

edited

Loading